Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Giving Roo's tests some love and attention #361

Merged
merged 9 commits into from
Dec 30, 2016

Conversation

stevendaniels
Copy link
Contributor

Contains the following changes:

  1. Refactored tests into different formats. (i.e. test_csv.rb, test_excelx.rb)
  2. Added rack app for testing download capabilities
  3. Fixed TestRoo#test_finalize

Refactored tests into different formats

This is the initial cleanup work for Roo's tests. Over the past year, I've
spent a lot of time thinking about #242, and I've written a lot of code
around implementing such an API. Unfortunately, I'd always end up
at a point where I wasn't happy with the either the test coverage of
the new API or the how the functionality between the two APIs was
mixed. As I said, I have a lot of partially written code.

The ultimate goal of the test refactoring is to allow me to create the
new API while also being able to take advantage of Roo's test suite.

Added Rack app for testing download capabilities

Added a rack app for downloading spreadsheet files. This allows Roo
to test downloading files without webmock or VCR.

###Fixed TestRoo#test_finalize

test_finalize broke in ruby 2.4 because of changes in Ruby's GC. In
Ruby 2.4, the last object being created didn't get GC'd, which was a
change from earlier behavior. The initial fix was to fork the process to
make sure directories where being cleaned properly.

Unfortunately, this caused a segfault when running the entire test suite.
The segfault issue was related to where ObjectSpace.define_finalizer
was being called from (a class that was being extended by a module).
This is probably related to Nokogiri, but I wasn't able to develop a trivial
example of the error.

Moving the finalizer's cration to Roo::Excelx/Roo::OpenOffice classes
fixed the issue in Ruby 2.4.0.

+ Removed some commented out tests
+ Moved tests requiring downloading to format specific files
+ Updated gemspec and Gemfiles
Also did some minor test refactoring
`test_finalize` broke in ruby 2.4 because of changes in Ruby's GC. The
initial fix was to fork the process to make sure directories where being
cleaned properly.

Unfortunately, this caused a segfault when running the entire test suite.
The issue was related to where `ObjectSpace.define_finalizer` was being
called from (a class that was being extended by a module). The issue is
probably related to Nokogiri, but I wasn't able to develop a trivial
example of the error.

Creating the finalizer in the class fixed the issue in Ruby 2.4.0.
@coveralls
Copy link

Coverage Status

Coverage increased (+0.06%) to 93.813% when pulling 6c1c1c3 on feature/refactor_tests into 143fffa on master.

3 similar comments
@coveralls
Copy link

Coverage Status

Coverage increased (+0.06%) to 93.813% when pulling 6c1c1c3 on feature/refactor_tests into 143fffa on master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.06%) to 93.813% when pulling 6c1c1c3 on feature/refactor_tests into 143fffa on master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.06%) to 93.813% when pulling 6c1c1c3 on feature/refactor_tests into 143fffa on master.

@stevendaniels stevendaniels changed the title Giving Roo's tests some love and attention Giving Roo's tests some love and attention ❤️ Dec 30, 2016
@stevendaniels stevendaniels changed the title Giving Roo's tests some love and attention ❤️ Giving Roo's tests some love and attention Dec 30, 2016
@coveralls
Copy link

coveralls commented Dec 30, 2016

Coverage Status

Coverage increased (+0.06%) to 93.813% when pulling 3690462 on feature/refactor_tests into 143fffa on master.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage increased (+0.06%) to 93.813% when pulling 3690462 on feature/refactor_tests into 143fffa on master.

@stevendaniels stevendaniels merged this pull request into master Dec 30, 2016
@stevendaniels stevendaniels deleted the feature/refactor_tests branch December 31, 2016 01:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants